Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: convert breakpoints variables to values instead of strings #1458

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

AxelPeter
Copy link
Contributor

@AxelPeter AxelPeter commented Aug 2, 2023

Changes description

Change value of Breakpoints design tokens from strings to values to be used in var()

Context

With Web Checkout, we try to rely the most on Vitamin design tokens. But i have an issue when using the breakpoints design tokens. Currently, they are strings and can't be used as value in var()

/* Don't work */
--foo: "900px";
max-width: var(--foo);

/* Works */
--foo: 900px;
max-width: var(--foo);

Checklist

  • Make sure you are requesting to pull a topic/feature/bugfix branch. Please, don't request directly from your main!
  • Check commits & PR names matches our requested structure. It must follow the https://www.conventionalcommits.org pattern.
  • Check your code additions will fail neither code linting checks.
  • I have reviewed the submitted code.
  • I have tested on related showcases.
  • If it includes design changes, please ask for a review design-system-core-team-design GitHub team.

Does this introduce a breaking change?

If these tokens are used as strings somewhere, it could be a breaking change.
But any others design tokens are values and not strings. I've checked in the codebase any use as strings but didn't found any, so it should not be a problem here

Can't be used in var() if they are strings
@AxelPeter AxelPeter added bug 🐛 Something isn't working good first issue Good for newcomers CSS 🎨 Related to CSS styles packages labels Aug 2, 2023
@AxelPeter AxelPeter requested review from thibault-mahe and a team as code owners August 2, 2023 09:04
@CLAassistant
Copy link

CLAassistant commented Aug 2, 2023

CLA assistant check
All committers have signed the CLA.

@lauthieb
Copy link
Member

lauthieb commented Aug 2, 2023

Thanks @AxelPeter for this PR, good catch!
I just checked and I think this will not have consequences for our existing consumers. So we can merge :)

@lauthieb lauthieb merged commit e0e3a34 into main Aug 2, 2023
7 checks passed
@lauthieb lauthieb deleted the fix/breakpoints-design-tokens branch August 2, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working CSS 🎨 Related to CSS styles packages good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants